Skip to content

Update to MTP #958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Nov 18, 2025
Merged

Update to MTP #958

merged 4 commits into from
Nov 18, 2025
+32 −67

Conversation

@Youssef1313
Copy link
Contributor

@Youssef1313 Youssef1313 commented Nov 11, 2025

Motivation and Context

Keep the test infra up-to-date with the latest dependencies and moving away from VSTest to the newer Microsoft.Testing.Platform.

How Has This Been Tested?

Manually running tests locally (still not working, that's why it's draft)

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Infrastructure and CI

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Youssef1313
Youssef1313 commented Nov 11, 2025
View reviewed changes
@stephentoub stephentoub marked this pull request as ready for review November 17, 2025 16:50
@Youssef1313
Copy link
Contributor Author

Youssef1313 commented Nov 17, 2025

@stephentoub This hopefully should be green.

@Youssef1313
Copy link
Contributor Author

Youssef1313 commented Nov 17, 2025 

failed ModelContextProtocol.Tests.Transport.StdioClientTransportTests.CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked (135ms)
  Assert.InRange() Failure: Value not in range
Range:  (1 - 2147483647)
Actual: 0
  from /home/runner/work/csharp-sdk/csharp-sdk/artifacts/bin/ModelContextProtocol.Tests/Debug/net472/ModelContextProtocol.Tests.exe (Mono 6.8.0.105 (Debian 6.8.0.105+dfsg-3.6ubuntu2 Sun Mar 31 02:55:28 UTC 2024)|x64)
  Assert.InRange() Failure: Value not in range
  Range:  (1 - 2147483647)
  Actual: 0

Is this a flaky test?

@stephentoub
Copy link
Contributor

stephentoub commented Nov 17, 2025 

failed ModelContextProtocol.Tests.Transport.StdioClientTransportTests.CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked (135ms)
  Assert.InRange() Failure: Value not in range
Range:  (1 - 2147483647)
Actual: 0
  from /home/runner/work/csharp-sdk/csharp-sdk/artifacts/bin/ModelContextProtocol.Tests/Debug/net472/ModelContextProtocol.Tests.exe (Mono 6.8.0.105 (Debian 6.8.0.105+dfsg-3.6ubuntu2 Sun Mar 31 02:55:28 UTC 2024)|x64)
  Assert.InRange() Failure: Value not in range
  Range:  (1 - 2147483647)
  Actual: 0

Is this a flaky test?

Probably

@Youssef1313
Copy link
Contributor Author

Youssef1313 commented Nov 18, 2025 •
edited
Loading

@stephentoub Can you please restart the failing jobs again? Looks like some flakiness.
Note: I ran this in my fork and it's green already Youssef1313#2

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Nov 18, 2025

@Youssef1313 done

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Nov 18, 2025

Thanks!

@eiriktsarpalis eiriktsarpalis merged commit 7080e3a into modelcontextprotocol:main Nov 18, 2025
12 of 14 checks passed
@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Nov 19, 2025

Hi @Youssef1313, we're getting reports of dotnet test failing after this update. I was able to repro this locally with the following error message:

The following exception occurred when running the test module with RunCommand 'C:\Users\eitsarpa\devel\mcp\csharp-sdk\artifacts\bin\ModelContextProtocol.Tests\net10.0\ModelContextProtocol.Tests.exe' and RunArguments '  --results-directory C:\Users\eitsarpa\devel\mcp\csharp-sdk\artifacts\TestResults\ --report-trx --report-trx-filename ModelContextProtocol.Tests.net10.0.Windows_NT.trx':

System.ComponentModel.Win32Exception (2): An error occurred trying to start process 'C:\Users\eitsarpa\devel\mcp\csharp-sdk\artifacts\bin\ModelContextProtocol.Tests\net10.0\ModelContextProtocol.Tests.exe' with working directory 'C:\Users\eitsarpa\devel\mcp\csharp-sdk'. The system cannot find the file specified.
   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
   at System.Diagnostics.Process.Start(ProcessStartInfo startInfo)
   at Microsoft.DotNet.Cli.Commands.Test.TestApplication.RunAsync()
   at Microsoft.DotNet.Cli.Commands.Test.TestApplication.RunAsync()
   at Microsoft.DotNet.Cli.Commands.Test.TestApplicationActionQueue.Read(BuildOptions buildOptions, TestOptions testOptions, TerminalTestReporter output, Action`1 onHelpRequested)

eiriktsarpalis added a commit that referenced this pull request Nov 19, 2025
@eiriktsarpalis
Revert "Update to MTP (#958)"
c7e21ed 
This reverts commit 7080e3a.
@eiriktsarpalis eiriktsarpalis mentioned this pull request Nov 19, 2025
Merged
eiriktsarpalis added a commit that referenced this pull request Nov 19, 2025
@eiriktsarpalis
Revert "Update to MTP (#958)" (#989)
bf0a65b 
This reverts commit 7080e3a.
Youssef1313 added a commit to Youssef1313/csharp-sdk that referenced this pull request Nov 19, 2025
@Youssef1313
Revert "Revert "Update to MTP (modelcontextprotocol#958)" (modelconte…
848fd47 
…xtprotocol#989)"

This reverts commit bf0a65b.
halter73 added a commit that referenced this pull request Nov 21, 2025
@halter73
Revert "Update to MTP (#958)"
fa4fdda 
This reverts commit 7080e3a.
halter73 added a commit that referenced this pull request Nov 21, 2025
@halter73
Revert "Update to MTP (#958)" (#1011)
707c083
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stephentoub stephentoub stephentoub approved these changes

@eiriktsarpalis eiriktsarpalis eiriktsarpalis approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Youssef1313 @stephentoub @eiriktsarpalis